Skip to content

Refactor: Remove some clippy allows - #199

Merged
ClausHolbechArista merged 2 commits into
aristanetworks:mainfrom
gmuloc:clippy-contained-removals
Jul 30, 2026
Merged

Refactor: Remove some clippy allows#199
ClausHolbechArista merged 2 commits into
aristanetworks:mainfrom
gmuloc:clippy-contained-removals

Conversation

@gmuloc

@gmuloc gmuloc commented Jul 29, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Change Summary

Continuing on the work to remove clippy allows that were added when adding workspace lint on all the crates

Checklist

Repository Checklist

  • My code has been rebased from main before I start
  • I have read the CONTRIBUTING document.
  • I have updated testing accordingly. (check the box if not applicable)

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of YAML value comparisons by using floating-point bitwise equality for round-trip checks.
    • Added more explicit panic messages when YAML writing fails or UTF-8 decoding encounters invalid data.

  • Tests

    • Refined validation error conversion logic in Python bindings tests to use clearer error-pattern handling.
    • Updated test lint configurations to reduce noise and better align with current lint expectations.

@gmuloc
gmuloc requested review from a team as code owners July 29, 2026 12:56
@gmuloc gmuloc added this to the v0.0.8 milestone Jul 29, 2026
@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6c26beb-3686-47df-84de-aea00c8559b5

📥 Commits

Reviewing files that changed from the base of the PR and between 9c2bcfb and cf0381a.

📒 Files selected for processing (1)
  • rust/yaml-parser/tests/writer_basic.rs
📝 Walkthrough

Walkthrough

Rust lint suppressions are reduced, Python binding error-handling syntax is modernized, and YAML writer tests use explicit failure handling and bitwise float comparisons.

Changes

Rust lint and test cleanup

Layer / File(s) Summary
Lint suppression cleanup
rust/avdschema/src/schema/dict/mod.rs, rust/python-bindings/src/lib.rs, rust/yaml-parser/tests/writer_basic.rs		 Removes obsolete Clippy allowances from schema documentation, Python bindings, and YAML writer tests.
Python binding error-handling cleanup
rust/python-bindings/src/schema_store/mod.rs, rust/python-bindings/src/tests/validation.rs		 Removes a trailing format! comma and replaces a validation-result match with let-else destructuring.
YAML roundtrip test cleanup
rust/yaml-parser/tests/writer_basic.rs		 Compares floats using to_bits() and replaces expect calls with explicit panic branches.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: rn: Refactor(rust)

Suggested reviewers: clausholbecharista

Poem

A rabbit trims the linty hay,
And lets-else hops lead the way.
Floats compare bit by bit,
Clear panics guard every split.
Rust tests bloom, neat and bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes only a brief summary and checklist, but omits several required template sections. Add the missing template sections: Related Issue(s), Component(s) name, Proposed changes, and How to test, and complete the checklist as needed.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: removing Clippy allow attributes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.76%. Comparing base (d5ee01d) to head (cf0381a).

Files with missing lines Patch % Lines
rust/python-bindings/src/schema_store/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #199   +/-   ##
=======================================
  Coverage   88.76%   88.76%           
=======================================
  Files          85       85           
  Lines       15613    15613           
=======================================
  Hits        13859    13859           
  Misses       1754     1754
Components Coverage Δ
Python 100.00% <ø> (ø)
Rust 88.74% <0.00%> (ø)
Files with missing lines Coverage Δ
rust/avdschema/src/schema/dict/mod.rs 98.38% <ø> (ø)
rust/python-bindings/src/lib.rs 100.00% <ø> (ø)
rust/python-bindings/src/schema_store/mod.rs 60.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai[bot]
coderabbitai Bot reviewed Jul 29, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/yaml-parser/tests/writer_basic.rs`:
- Around line 123-129: Update the error branches around write_yaml_from_events
and String::from_utf8 to bind each Err value and include it with {:?} in the
panic messages, preserving the underlying writer and UTF-8 conversion errors
while retaining the existing success behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3e13df8c-d589-49d0-a420-3203d6ec2a08

📥 Commits

Reviewing files that changed from the base of the PR and between d5ee01d and 9c2bcfb.

📒 Files selected for processing (5)
  • rust/avdschema/src/schema/dict/mod.rs
  • rust/python-bindings/src/lib.rs
  • rust/python-bindings/src/schema_store/mod.rs
  • rust/python-bindings/src/tests/validation.rs
  • rust/yaml-parser/tests/writer_basic.rs
💤 Files with no reviewable changes (1)
  • rust/python-bindings/src/lib.rs

Comment thread rust/yaml-parser/tests/writer_basic.rs Outdated
@gmuloc

gmuloc commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@ClausHolbechArista This is uncovered before refactor so Codecov failure can be ignored

@ClausHolbechArista
ClausHolbechArista merged commit 92b331b into aristanetworks:main Jul 30, 2026
101 of 103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

Assignees

@gmuloc gmuloc

Labels

rn: Refactor

Projects

None yet

Milestone

v0.0.8

Development

Successfully merging this pull request may close these issues.

2 participants

@gmuloc @ClausHolbechArista